Use regular imports instead of require where possible#59017
Use regular imports instead of require where possible#59017jakebailey merged 3 commits intomicrosoft:mainfrom
Conversation
|
@typescript-bot perf test this |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import childProcess from "child_process"; | ||
| import fs from "fs"; | ||
| import net from "net"; | ||
| import os from "os"; | ||
| import readline from "readline"; |
There was a problem hiding this comment.
Is there any risk now that these are unconditional imports?
There was a problem hiding this comment.
Nope, this is the tsserver project which only runs in node.
| let cancellationToken: ts.server.ServerCancellationToken; | ||
| try { | ||
| const factory = require("./cancellationToken"); | ||
| const factory = require("./cancellationToken.js"); |
There was a problem hiding this comment.
Hi, I came across this line running dependency-cruiser on the typescript codebase. After building, it's the only require/import that can't be resolved. I notice that this file, nodeServer, does not have a cancellationToken sibling file - is it possible that this is always throwing (seems like it could be, since the catch block silently catches the error and uses ts.server.nullCancellationToken)
There was a problem hiding this comment.
This file should exist as a sibling. https://unpkg.com/browse/typescript@5.6.3/lib/cancellationToken.js
Are you running this tool on the built code, or the source?
There was a problem hiding this comment.
(realistically, it's not clear to me why this is a separate file at all anyway)
There was a problem hiding this comment.
Oh I see. I'm running it on the source, so I guess it's missing some part of the build process that compiles src/tsserver/nodeServer.ts into lib/tsserver.js, and src/cancellationToken/cancellationToken.ts into lib/cancellationToken.js, is that right? (Something in Hereby?)
Would it not work as require("../cancellationToken/cancellationToken.js") (which I think is what would be correct for the source file structure)
There was a problem hiding this comment.
No, that would not work. Our input source structure has nothing to do with our output source structure; it's bundled and placed in a different directory.
This is pulled out of #58419, since I think this is generally better and we can do it now.